Test fails on 0.7, not because of this PR though:

EventListener: Test Failed at /home/travis/.julia/v0.7/GitHub/test/event_tests.jl:13
  Expression: GitHub.has_valid_secret(event_request, "secret")
Stacktrace:
 [1] macro expansion at /home/travis/.julia/v0.7/GitHub/test/event_tests.jl:13 [inlined]
 [2] macro expansion at /buildworker/worker/package_linux64/build/usr/share/julia/site/v0.7/Test/src/Test.jl:1022 [inlined]
 [3] top-level scope at /home/travis/.julia/v0.7/GitHub/test/event_tests.jl:12
EventListener: Error During Test at /home/travis/.julia/v0.7/GitHub/test/event_tests.jl:18
  Expression evaluated to non-Boolean
  Expression: GitHub.handle_event_request(event_request, (x->begin
            true
        end), secret="secret", events=["commit_comment"], repos=["JuliaCI/BenchmarkTrackers.jl"])
       Value: HTTP.Messages.Response:
"""
HTTP/1.1 400 Bad Request
invalid signature"""

I'm running Pkg.test("GitHub.jl") with this branch on my local machine (macOS v0.6.2) now...

I've modified runtests.jl like this to see if the connection pooling / timeout is working:

for i in 1:10
    include("read_only_api_tests.jl")
    HTTP.ConnectionPool.showpool(STDOUT)
    sleep(40)
end

But the read_only_api_tests does not start any server so should be unaffected by this? Maybe I misunderstood something about where I should put this keyword, I thought it was a server setting (I know nothing about this stuff :D).

You will need REQUIRE: HTTP 0.6.8 too (not tagged yet).

ok, no, the idle_timeout=20 is a request option.

Updated to be a request option.

That seems to be working (note that the local socket number is different after the 2nd run):

Test Summary:       | Pass  Total
Tags and References |    1      1
ConnectionPool[
   ⏸   63↑    63↓    api.github.com:443:53937 ≣16
]

Test Summary: | Pass  Total
Owners        |    9      9
Test Summary: | Pass  Total
Repositories  |   16     16
Test Summary: | Pass  Total
Issues        |    4      4
Test Summary: | Pass  Total
Gists         |    5      5
Test Summary: | Pass  Total
Reviews       |    1      1
Test Summary: | Pass  Total
Activity      |    4      4
Test Summary: | Pass  Total
Apps          |    4      4
Test Summary: | Pass  Total
Git Data      |    4      4
Test Summary:       | Pass  Total
Tags and References |    1      1
ConnectionPool[
   ⏸   63↑    63↓    api.github.com:443:53939 ≣16
]

Before the change I aws seeing this:

Test Summary:       | Pass  Total
Tags and References |    1      1
ConnectionPool[
   ⏸   63↑    63↓    api.github.com:443:53854 ≣16
]

Test Summary: | Pass  Total
Owners        |    9      9
Test Summary: | Pass  Total
Repositories  |   16     16
Test Summary: | Pass  Total
Issues        |    4      4
Test Summary: | Pass  Total
Gists         |    5      5
Test Summary: | Pass  Total
Reviews       |    1      1
Test Summary: | Pass  Total
Activity      |    4      4
Test Summary: | Pass  Total
Apps          |    4      4
Test Summary: | Pass  Total
Git Data      |    4      4
Test Summary:       | Pass  Total
Tags and References |    1      1
ConnectionPool[
   ⏸  126↑   126↓    api.github.com:443:53854 ≣16
]

Ok, @ararslan, should we make Nanosoldier a guinea pig for this (in other words update HTTP and use this PR) or what do you prefer?

I'm just reviewing the diffs since last HTTP.jl tag now. I think I'm ok to tag, but I might be better to wait for @quinnj's input. It would probably be best to manually checkout this branch of GitHub.jl and master of HTTP.jl on nanosolidier to test.

should we make Nanosoldier a guinea pig for this (in other words update HTTP and use this PR) or what do you prefer?

That seems fine to me. There's actually a way to set up a test server for Nanosoldier but it's way more effort than just subjecting people to testing in production, so..................

I've reviewed the recent HTTP.jl changes. All the significant changes were already well reviewed in their respective PRs, and only take effect when new options are enabled, so it they seem pretty low risk. I've tagged HTTP.jl https://github.com/JuliaWeb/HTTP.jl/releases/tag/v0.6.8
But, I've left a note on the METADATA.jl PR to request that it not be merged right away.

Nanosoldier seems to be running well using HTTP master and this branch of GitHub. There have only been a handful of triggered runs since this was set up, but there's been no ignoring and no IOErrors.

@ararslan it would be a good idea to update the nanosolider box to use latest HTTP master to test this change: JuliaWeb/HTTP.jl@fa6b206
The change reverts one of the things we tried in an attempt to reduce the nanosolider post retry problem. I believe that with GitHub.jl now using idle_timeout the original change is redundant (and it appears to cause problems elsewhere).

Should this be merged first?

This looks good to be merged to me (but I don't have commit access here).

See JuliaCloud/AWSS3.jl#26 (comment)
The reporter of the AWSS3 issue has found that the HTTP.jl change fixes his problem.

Looks like travis passes for 0.6 but not for 0.7.

New HTTP version is tagged and waiting for METADATA PR: JuliaLang/METADATA.jl#14286

Passes on 0.7 locally now but needs new release of MbedTLS (JuliaLang/MbedTLS.jl#137)

What's the status of this?

Not sure if this is still needed after all the new stuff in HTTP.jl.

I think if the github API is known to dump connections after x seconds, then having idle_timeout=x * some_fraction is the right thing to do, event if other changes in HTTP.jl improved the automatic retry behaviour.
This PR seems to have a bunch of 0.7 compat stuff too.

A few more dep warnings now but this is a step in the right direction.